Skip to content

feat(@angular-devkit/build-angular): default to NodeJS value for pres… #16648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

Toxicable
Copy link

…erveSymlinks

Under bazel preserveSymlinks would have to be set in two different places, this makes it so it only has to be set once by using the value from NodeJS if it's set.

@Toxicable
Copy link
Author

Not sure if this is the right place to put this config, let me know if there's a better place to set it.

@clydin
Copy link
Member

clydin commented Jan 17, 2020

I’m ok with using it as a default. However, it would be a breaking change since the default would be different for some people. This could manifest in either broken builds or builds containing different dependencies than anticipated or expected. As a feature, this would probably fall into the v10 timeframe.

@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch 2 times, most recently from e3699cb to f5d55dd Compare January 17, 2020 05:52
@Toxicable
Copy link
Author

@clydin Sounds good to me, how would we make sure to come back and check this when there's a v10 branch?

@alan-agius4
Copy link
Collaborator

@Toxicable, I can create a v10 milestone and assign this Pr to it.

@alan-agius4 alan-agius4 added this to the V10-candidates milestone Jan 17, 2020
@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch 2 times, most recently from 826bdc6 to 6105bf4 Compare January 17, 2020 07:16
@alan-agius4 alan-agius4 changed the title feat(@angualr-devkit/build-angular): default to NodeJS value for pres… feat(@angular-devkit/build-angular): default to NodeJS value for pres… Jan 17, 2020
@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch from 6105bf4 to 3282882 Compare January 17, 2020 19:27
@Toxicable Toxicable force-pushed the preserve-symlink-nodejs branch from 3282882 to 611a57c Compare January 17, 2020 19:29
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Fabian! LGTM.

@alan-agius4 alan-agius4 self-assigned this Jan 17, 2020
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release and removed PR state: blocked labels Mar 23, 2020
@dgp1130 dgp1130 merged commit bc5ce39 into angular:master Mar 23, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants